-
Notifications
You must be signed in to change notification settings - Fork 521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed #1769 Suggest to rename the image name #2245
base: master
Are you sure you want to change the base?
Conversation
Eclipse JKube CI ReportStarted new GH workflow run for #2245 (2023-09-01T08:20:32Z) ⚙️ JKube E2E Tests (6046868103)
|
Codecov Report
@@ Coverage Diff @@
## master #2245 +/- ##
============================================
+ Coverage 59.36% 59.86% +0.50%
- Complexity 4586 4611 +25
============================================
Files 500 505 +5
Lines 21211 21043 -168
Branches 2830 2785 -45
============================================
+ Hits 12591 12597 +6
+ Misses 7370 7231 -139
+ Partials 1250 1215 -35
... and 8 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
String message = "Error while trying to push the image: " + ex.getMessage() + | ||
"\nPossible issue: wrong image name or registry." + | ||
"\nHint: Rename image name or registry with the jkube.generator.name property = registry name and user name and image name"; | ||
throw new JKubeServiceException(message, ex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should always log this warning. IOException could also mean problems with network. It would be better if we could rely on docker response codes. If not you can check the exception message for keywords like unauthorized
/denied
etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need to do the same for JibBuildService as well.
@rohanKanojia is it okay now? |
Kudos, SonarCloud Quality Gate passed! |
@sunix can you please review and merge this PR? |
Sun is on PTO. However, did Rohan approve based on the changes you made? (if not, please don't ask someone else to 'merge') |
I made the changes that he suggested. Didn't get a reply from @rohanKanojia on this. I'll check with him again. |
Don't worry. I spoke with him yesterday. He will review when available. Just making sure we do not miss addressing comments. |
@rohanKanojia should I squash the commits on this PR? |
I tested it on DockerHub, Quay.io and GitHub Container Registry, it seems to be working okay. I think if a user gets unauthorized error, there are two possibilities:
I think we should print two hints instead of one. |
Could you please add some unit tests for this? Maybe we should try to move this logic to some common place to avoid duplication. |
...ervice/src/main/java/org/eclipse/jkube/kit/config/service/kubernetes/DockerBuildService.java
Outdated
Show resolved
Hide resolved
1f43d07
to
528af16
Compare
TODO for this issue: debug with docker and jib, extract the error and create unit test based on the extracted error. |
Kudos, SonarCloud Quality Gate passed! 0 Bugs 0.0% Coverage The version of Java (11.0.18) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. |
…sh task fails
Description
Fixes #1769
Type of change
test, version modification, documentation, etc.)
Checklist